Skip to content

Fix SSO wire format (id key, ms timestamp, camelCase payload)#6

Merged
winrid merged 2 commits into
mainfrom
fix/sso-wire-format
Jul 2, 2026
Merged

Fix SSO wire format (id key, ms timestamp, camelCase payload)#6
winrid merged 2 commits into
mainfrom
fix/sso-wire-format

Conversation

@winrid

@winrid winrid commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

What

Fixes the hand-written sso/ module so it produces tokens the FastComments server actually accepts. The generated client/ code is untouched.

The bugs (all in sso/, not generated code)

Verified against the server verifier fastcomments/util/sso-utils.ts (decryptSSOPayload):

  • SecureSSOUserData serialized the id as user_id instead of id -> tokens failed with missing-user-id.
  • FastCommentsSSO.new_secure used int(time.time()) (seconds) instead of milliseconds -> outside the server's timestamp window.
  • SecureSSOPayload.toJSON emitted snake_case keys (user_data_json_base64, verification_hash) instead of the wire format userDataJSONBase64 / verificationHash / timestamp.
  • SimpleSSOUserData omitted username entirely (the simpleSSO object is keyed on it).

Why the tests didn't catch it

The old tests/test_sso.py asserted the buggy behavior against itself: it checked parsed["user_id"] and snake-case parsed["user_data_json_base64"], tested create_verification_hash with an arbitrary timestamp (so ms-vs-seconds never surfaced), and built simple SSO without a username. The only test that exercises the real end-to-end path (posting a token to the server) is the integration suite, which is skipped without live credentials. The unit tests were internally consistent with the wrong implementation.

Changes

  • Correct all four issues above; id is stringified and null fields are dropped.
  • Add the optional Secure SSO fields (avatar, displayName, websiteUrl, groupIds, isAdmin, isModerator, ...), plus login_url/logout_url plumbing and SecureSSOPayload.to_widget_dict() for embedding directly in a widget config.
  • Move the generated client's runtime deps (pydantic, urllib3, python-dateutil, typing-extensions) into a client extra, so the base install is pure-stdlib SSO. Install the client with pip install "fastcomments[client] @ git+...".
  • Fix README examples (id= kwarg, api_key auth key, host without the erroneous /api suffix).
  • Rewrite the unit tests to assert the real wire format, add a pinned cross-SDK parity vector, make unit tests run offline, and gate integration tests on real credentials.

Verification

  • pytest tests/: 22 passed, 7 skipped (integration needs credentials).
  • Cross-language check: a Python-signed token is recomputed by the server's exact HMAC algorithm (Node) and matches; the decoded user has id (not user_id) and a millisecond timestamp.

This is a prerequisite for the new fastcomments-django integration, which depends on this module for SSO signing.

The hand-written sso module produced tokens the FastComments server rejects
(verified against server util/sso-utils.ts decryptSSOPayload):

- SecureSSOUserData serialized `user_id` instead of `id`, so tokens failed
  with missing-user-id
- FastCommentsSSO.new_secure used epoch seconds instead of milliseconds
- SecureSSOPayload emitted snake_case keys (user_data_json_base64) instead of
  the widget/server format userDataJSONBase64 / verificationHash
- SimpleSSOUserData omitted username entirely

Also:
- Add the optional Secure SSO fields (avatar, displayName, websiteUrl,
  groupIds, isAdmin, isModerator, ...), null-stripping, and login/logout URLs;
  add SecureSSOPayload.to_widget_dict() for embedding in a widget config
- Move the generated client's runtime deps (pydantic, urllib3, ...) into a
  `client` extra so the base install is pure-stdlib SSO
- Fix README examples (id kwarg, api_key auth key, host without /api)
- Rewrite the unit tests to assert the real wire format and add a pinned
  cross-SDK parity vector; make unit tests run offline and gate the
  integration tests on real credentials
@winrid
winrid requested review from CarsonBurke July 2, 2026 05:01
Correcting the DefaultApi auth key means the request now authenticates and
reaches user validation, which returns 422 for the made-up context_user_id
(previously the wrong auth key returned 401). Accept 422 alongside the other
expected statuses.
@winrid
winrid merged commit 57b7908 into main Jul 2, 2026
1 check passed
@winrid
winrid deleted the fix/sso-wire-format branch July 2, 2026 05:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant